I agree, there are some things that could be tidied up a bit still. I think before I fix up these little ones we should decide if the auto-formatting is something we want more generically or not?

After this PR I also tested out black, which surprisingly wasn't as bad as I expected it to be... main...greglucas:black
So, I may even prefer to go that direction and just be done with any formatting questions. Some of the examples have obnoxious list expansion that we could ignore, but most of the updates seem pretty reasonable. My suggestion would be to swap autoflake8 to black in the pre-commit integration here if others are also interested in that swap.

If it helps, several other SciTools projects have adopted black, and I think we’re pretty happy about not having to think about the formatting any more.

@greglucas As a workflow, I'd prefer to see cartopy use the pre-commit.ci service to run all registered pre-commit git-hooks as part of the pull-request workflow 👍

IMHO this is a light-weight way to enforce compliance and ensure code quality across the repo.

@greglucas Could you also adopt the use of .git-blame-ignore-revs in this PR, so that you are not blamed for other authors contributions due to your flake8 compliance changes? Such PRs like this are typically noisy and .git-blame-ignore-revs helps mitigate against this.

You can also decorate the commit sha with a comment to record some provenance to what the commit sha is, what PR it's associated with and the change that was introcuded e.g., something along the lines of...

.git-blame-ignore-revs:

# style: flake8 (#1934)
7c86bc0168684345dc475457b1a77dadc77ce9bb

(I've used a bogus sha here)

I think this would be super useful 👍

@greglucas Are you happy for me to enable pre-commit.ci for cartopy? I've got the ownership perms to do that...

I would be in favor of using pre-commit.ci. One thing I have run into on other projects is that if you enable it and we don't go with it or this PR sits for a while, it will fail on the main branch annoyingly because the configuration isn't present on that branch. Maybe that was because it was in a different branch though and not a fork... I don't remember specifically what was going on just that there were a lot of red x's that weren't actually failures.

I would be in favor of using pre-commit.ci. One thing I have run into on other projects is that if you enable it and we don't go with it or this PR sits for a while, it will fail on the main branch annoyingly because the configuration isn't present on that branch.

Well, I'm happy to merge this PR now and enable pre-commit.ci

I guess that might force other PR authors to rebase once this PR lands on main, but I'm seeing the advantage of this PR far out-weighing the rebase hassle factor in the long-run.

Thoughts?

Are you happy for me to bank this and see how we roll?

Note to self: use a merge commit and not squash and merge

I guess that might force other PR authors to rebase once this PR lands on main

We have a lot of PRs that have been sitting for several years now, so I'm guessing rebasing is expected at this point 😄

I'm fine with rolling with it and making updates incrementally as we go if this does cause any issues.

@greglucas Tis a beautiful thing...

Nice one mate! 🍻

@greglucas I'll push a PR to add the pre-commit.ci badge to the README.md... inbound now.